Skip to content

Conversation

silvanshade
Copy link
Contributor

I created a small example using the generated WebKit bindings. If you have any ideas on how to improve the structure, feel free to make whatever changes you'd like.

I also made some feature sets for the examples so that they can be run a little easier and I added a README.

@silvanshade
Copy link
Contributor Author

silvanshade commented Jan 19, 2023

EDIT: I think I figured out most of the issues I mentioned (but feedback still welcome)

Copy link
Owner

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please, examples are very much welcome!

@silvanshade
Copy link
Contributor Author

One observation I had about writing the init method is that it would be nice if we could somehow use ? but I wasn't sure if it was possible to return a Result here:

unsafe fn __init_withTextField_andWebView(self: &mut Self, text_field: *mut NSTextField, web_view: *mut WKWebView) -> Option<&mut Self> {
    let this: Option<&mut Self> = msg_send![super(self), init];
    this.and_then(|this| {
        Id::retain(text_field).and_then(|text_field| {
            Id::retain(web_view).and_then(|web_view| {
                Ivar::write(&mut this.text_field, text_field);
                Ivar::write(&mut this.web_view, web_view);
                Some(this)
            })
        })
    })
}

The .and_then chaining feels a little clunky though. Ideally Id::retain would return a Result there too then, or maybe we could have some variant of it that does (since writing .ok_or(<some long-ish reason>) repeatedly is still verbose IMO).

If try_blocks were stable, that would also work nicely here if returning Result is generally problematic, since we could just write a try-block and terminate with .ok().

@madsmtm
Copy link
Owner

madsmtm commented Jan 26, 2023

One observation I had about writing the init method is that it would be nice if we could somehow use ?

You can use ? on Option<T>.

After #173, you would even be able to write:

#[method_id(initWithTextField:andWebView:)]
unsafe fn __init_withTextField_andWebView(this: Allocated<Self>, text_field: *mut NSTextField, web_view: *mut WKWebView) -> Option<Id<Self, Owned>> {
    let this = unsafe { msg_send_id![super(self), init] }?;
    Ivar::write(&mut this.text_field, unsafe { Id::retain(text_field) }?);
    Ivar::write(&mut this.web_view, unsafe { Id::retain(web_view) }?);
    Some(this)
}

Also, I have ideas for making Id::retain safe for some cases, see #399.

Copy link
Owner

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing the unsafe change; I know it looks ugly, but hopefully with #359 and the property stuff in #330, things should get a lot better!

@silvanshade
Copy link
Contributor Author

You can use ? on Option<T>.

Very interesting. Did this change recently? I thought it used to not work (or it kind of did, but there was some problem involving NoneError)?

@madsmtm
Copy link
Owner

madsmtm commented Jan 27, 2023

You can use ? on Option<T>.

Very interesting. Did this change recently? I thought it used to not work (or it kind of did, but there was some problem involving NoneError)?

Well, recently is subjective, but godbolt shows it worked since Rust 1.22 ;)

@madsmtm madsmtm added documentation Improvements or additions to documentation A-framework Affects the framework crates and the translator for them labels Jan 27, 2023
@silvanshade silvanshade requested a review from madsmtm January 28, 2023 02:44
@silvanshade silvanshade requested a review from madsmtm January 29, 2023 21:43
Copy link
Owner

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks!

text_field: *mut NSTextField,
web_view: *mut WKWebView,
) -> Option<&mut Self> {
let this: Option<&mut Self> = msg_send![super(self), init];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's weird, I wouldn't have expected you to be able to elide the unsafe around this message send when we have #![deny(unsafe_op_in_unsafe_fn)]. But that's a different issue

@madsmtm madsmtm merged commit 7e702d2 into madsmtm:master Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-framework Affects the framework crates and the translator for them documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants